Repository navigation
Recognize localized AM/PM markers in parse_time() - #1426
Open
jeff-grimes-pplx wants to merge 1 commit into
Open
jeff-grimes-pplx wants to merge 1 commit into
jeff-grimes-pplx wants to merge 1 commit into
Conversation
parse_time() decided between AM and PM by searching the input for the English substrings "am" and "pm". Times formatted by Babel itself for locales whose CLDR day-period names are not English - Korean "오후", Spanish "p.m.", Greek "μ.μ.", Arabic "م", Chinese "下午", Albanian "m.d." and many more - were therefore parsed as AM, silently shifting every afternoon time back by 12 hours. Look the marker up in the locale's own AM/PM day-period names (all contexts and widths, longest name first, ignoring case and whitespace) and keep the English markers as a fallback.
Member
|
I'm sorry, how does #1168 relate to this? |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #1168.
Problem
babel.dates.parse_time()decides between AM and PM by searching the input for the English substrings"am"/"pm". Babel's ownformat_time()output for locales whose CLDR day-period names aren't English (Korean오후, Spanishp.m., Greekμ.μ., Arabicم, Chinese下午, Albanianm.d., Afarcarra, ...) is therefore always parsed as AM, so every afternoon time comes back 12 hours early. No error is raised.When I round-tripped
format_time()throughparse_time()for every CLDR locale whoseshort/mediumtime pattern uses theaperiod field, every hour 0–23, 4,038 of 17,040 round-trips were wrong, across 171 locales.Root cause
parse_time()(babel/dates.py, theif 'a' in format_str:block) hardcodes English markers:It never reads the locale's
day_periodsdata, even though the same data drivesformat_time()'safield.Fix
A small helper,
_parse_period(), works out the marker from the locale's own AM/PM day-period names:format/stand-alone) and every width (abbreviated/wide/narrow);pcan't match inside a longer one (dop.in Slovenian, for example);p. m., for example);am/pm, so existing callers that parse English markers with another locale keep working.The 12 AM / 12 PM normalisation is unchanged.
Verification
tests/test_dates.py::test_parse_time_localized_am_pm: it fails onmaster(assert datetime.time(3, 30) == datetime.time(15, 30)) and passes with the fix.7939 passed, 8 skipped, 2 xfailed.ruff checkis clean.h:mm:ss a,h:mm:ss aaaa,h:mm:ss aaaaa,a h:mm,K:mm a) and all 24 hours: correct round-trips go from 99,075 to 134,476 out of 134,520, with 0 regressions. The 44 that still fail are all Anii (blo), whose period names contain digits (1ka/2ja) and confuse the existing digit extraction. That's a separate, pre-existing limitation.B, used byzh_Hant, for example).